Skip to content

fix(native): resolve monorepo workspace package imports in the native engine#2061

Open
carlos-alm wants to merge 1 commit into
fix/issue-1923-complexity-metrics-cognitive-cyclomaticfrom
fix/issue-1927-native-engine-monorepo-workspace-package
Open

fix(native): resolve monorepo workspace package imports in the native engine#2061
carlos-alm wants to merge 1 commit into
fix/issue-1923-complexity-metrics-cognitive-cyclomaticfrom
fix/issue-1927-native-engine-monorepo-workspace-package

Conversation

@carlos-alm

Copy link
Copy Markdown
Contributor

Summary

resolve_import_path_inner in the native (Rust) engine had no workspace-awareness at all: a bare monorepo-package specifier (e.g. import "@myorg/lib") fell straight through to the raw-specifier fallback under native, unlike the WASM/JS engine's resolveViaWorkspace(), which resolves it to the package's real entry file — and grants the resulting cross-package call edges a 0.95 confidence floor via _workspaceResolvedPaths.

Changes

  • crates/codegraph-core/src/domain/graph/resolve.rs: ports resolveViaWorkspace()/parseBareSpecifier() (resolve_via_workspace, parse_bare_specifier), threaded through resolve_import_path/resolve_imports_batch. Adds a process-lifetime workspace-resolved-paths cache mirroring _workspaceResolvedPaths in resolve.ts, read by compute_confidence() to grant the same 0.95 floor natively — with no changes needed to compute_confidence's call sites.
  • crates/codegraph-core/src/lib.rs / db/connection.rs / domain/graph/builder/pipeline.rs / domain/graph/builder/stages/import_edges.rs: threads the already-detected workspace map through both call paths — the per-call FFI functions (resolve_import/resolve_imports) and the full Rust build orchestrator (NativeDatabase::build_graphrun_pipelineresolve_pipeline_imports/barrel re-parse/ImportEdgeContext).
  • crates/codegraph-core/src/types.rs: new WorkspacePackage napi/serde type ({packageName, dir, entry}), reused for both the FFI array parameter and the workspaces_json blob deserialized by the orchestrator (needs its own #[serde(rename_all = "camelCase")], same reason BuildPathAliases exists alongside PathAliases).
  • src/domain/graph/resolve.ts: new getWorkspacesForNative() helper converts the JS-side workspace map (populated by detectWorkspaces() — no native equivalent, matching the established config.rs JSON-handoff pattern) into the FFI shape; wired into resolveImportPath()/resolveImportsBatch().
  • src/domain/graph/builder/stages/native-orchestrator.ts / src/types.ts: new workspacesJson argument on buildGraph().
  • Tests: new tests/fixtures/monorepo-workspace/ fixture wired into the existing native/WASM build-parity.test.ts hard gate, plus a dedicated tests/integration/issue-1927-native-workspace-resolution.test.ts asserting the resolved import edge, its target file, and the 0.95 confidence floor for both engines. 21 new Rust unit tests cover parse_bare_specifier, resolve_via_workspace, resolve_non_relative_import, resolve_import_path, compute_confidence, and the cache reset contract.

Scope note

The native engine still has no package.json exports-field resolver at all (resolveViaExports() has no Rust counterpart — a separate, pre-existing, broader gap that also affects plain node_modules bare specifiers, not just workspace packages). resolve_via_workspace therefore skips that lookup and falls straight to main/source/index-file resolution, same as a repo without an exports field would get under either engine. Filed as #2060 — out of scope for this fix.

Test plan

  • cargo test (codegraph-core): 640 passed, 0 failed (21 new)
  • cargo clippy: no new warnings on touched files
  • npx tsc --noEmit: clean
  • npm run lint (Biome): clean
  • npx vitest run tests/unit/resolve.test.ts: 67 passed (pre-existing JS-side coverage, unaffected)
  • npx vitest run tests/integration/issue-1927-native-workspace-resolution.test.ts: 6 passed — both wasm and native engines resolve the workspace import to the same target file and grant the same 0.95 confidence
  • npx vitest run tests/integration/build-parity.test.ts -t monorepo-workspace: 4 passed — native and WASM produce byte-identical nodes/edges/roles/ast_nodes for the new fixture
  • Native addon rebuilt locally (napi build --platform --release, codesigned) and verified directly against the exact repro shape from the issue
  • Full npm test: run twice against this worktree's local WASM grammar cache (a pre-existing, only-partially-built cache unrelated to this change); both runs produced the identical 213/4103 failure set, 100% attributable to missing grammars for languages this fix does not touch (objc, julia, swift, elixir, dynamic-groovy, dynamic-scala precision/recall thresholds, plus the corresponding tests/parsers/*.test.ts and tests/engines/parity.test.ts files for those same languages) — confirmed via THRESHOLDS[lang] in resolution-benchmark.test.ts that every other 0%-coverage language already has an intentionally low/zero threshold unrelated to grammar availability. No test touching import/workspace resolution, confidence scoring, or the native build orchestrator failed.

Closes #1927

… engine

resolve_import_path_inner had no workspace-awareness at all: a bare
monorepo-package specifier (e.g. import "@myorg/lib") fell straight through
to the raw-specifier fallback under the native engine, unlike the WASM/JS
engine's resolveViaWorkspace(), which resolves it to the package's real
entry file and grants the resulting call edges a 0.95 confidence floor.

- Port resolveViaWorkspace()/parseBareSpecifier() into
  crates/codegraph-core/src/domain/graph/resolve.rs (resolve_via_workspace,
  parse_bare_specifier), threaded through resolve_import_path/
  resolve_imports_batch and both the per-call FFI path (lib.rs) and the full
  Rust build orchestrator (pipeline.rs, import_edges.rs).
- Add a process-lifetime workspace-resolved-paths cache in resolve.rs,
  mirroring _workspaceResolvedPaths in resolve.ts, so compute_confidence
  grants the same 0.95 confidence floor natively.
- Thread the already-detected workspace map (from detectWorkspaces() in
  infrastructure/config.ts — no native equivalent, matching the existing
  config.rs JSON-handoff pattern) across the FFI boundary: new
  NativeWorkspacePackage/WorkspacePackage type, new optional parameters on
  resolveImport/resolveImports/buildGraph, and a new getWorkspacesForNative()
  helper in resolve.ts.
- Add a monorepo-workspace fixture and wire it into the native/WASM
  build-parity hard gate, plus a dedicated regression test asserting the
  resolved edge, its target file, and the 0.95 confidence floor for both
  engines.

Native still has no package.json "exports" field resolver (a separate,
broader gap tracked in #2060) — resolve_via_workspace falls back to
main/source/index-file resolution for that case, same as a repo without an
exports field would get under either engine.
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Codegraph Impact Analysis

14 functions changed32 callers affected across 8 files

  • NativeOrchestrationSession in src/domain/graph/builder/stages/native-orchestrator.ts:2398 (4 transitive callers)
  • NativeOrchestrationSession.runBuildGraph in src/domain/graph/builder/stages/native-orchestrator.ts:2430 (4 transitive callers)
  • getWorkspacesForNative in src/domain/graph/resolve.ts:232 (23 transitive callers)
  • resolveImportPath in src/domain/graph/resolve.ts:611 (25 transitive callers)
  • resolveImportsBatch in src/domain/graph/resolve.ts:667 (3 transitive callers)
  • NativeWorkspacePackage.packageName in src/types.ts:2325 (0 transitive callers)
  • NativeWorkspacePackage.dir in src/types.ts:2326 (0 transitive callers)
  • NativeWorkspacePackage.entry in src/types.ts:2327 (0 transitive callers)
  • NativeAddon.resolveImport in src/types.ts:2335 (0 transitive callers)
  • NativeAddon.resolveImports in src/types.ts:2342 (0 transitive callers)
  • NativeDatabase.buildGraph in src/types.ts:2993 (0 transitive callers)
  • calculate in tests/fixtures/monorepo-workspace/apps/web/index.js:3 (0 transitive callers)
  • add in tests/fixtures/monorepo-workspace/packages/lib/src/index.js:1 (0 transitive callers)
  • multiply in tests/fixtures/monorepo-workspace/packages/lib/src/index.js:5 (0 transitive callers)

@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR ports the JS/WASM resolveViaWorkspace logic to the native Rust engine so that bare monorepo-package specifiers (e.g. import "@myorg/lib") resolve to the package's real entry file and receive the 0.95 workspace-confidence floor via a process-lifetime Mutex<HashSet> cache that mirrors _workspaceResolvedPaths in resolve.ts.

  • resolve.rs: new parse_bare_specifier, resolve_via_workspace, and workspace_resolved_cache (reset once per build, read by compute_confidence); the global-cache design is deliberately documented and matches the JS side's module-level variable contract.
  • pipeline.rs / connection.rs / import_edges.rs / lib.rs: workspace map threaded through both the per-call FFI path (resolve_imports resets cache before batch) and the full Rust orchestrator path (pipeline_setup resets before Stage 6).
  • Tests: 21 new Rust unit tests cover every new function; a new integration test and a build-parity.test.ts fixture assert identical native/WASM output for the two-package monorepo fixture.

Confidence Score: 4/5

Safe to merge; the workspace resolution logic is correct, all pipeline paths receive the workspace map, and 21 targeted Rust unit tests plus a parity gate guard the new behavior.

The core algorithm and cache-reset contract are sound and well-covered. The two findings are both non-blocking: the integration test's confidence assertion is satisfied by the 1.0 path rather than the intended 0.95 floor (a coverage gap, not a behavioral regression), and the silent skip on a poisoned mutex in reset is a theoretical concern that requires a panic-while-locked scenario to manifest.

resolve.rs — the process-lifetime workspace cache and its reset contract are the most critical logic to re-read; the test file issue-1927-native-workspace-resolution.test.ts should have its confidence assertion revisited if the 0.95 floor specifically needs integration-level coverage.

Important Files Changed

Filename Overview
crates/codegraph-core/src/domain/graph/resolve.rs Ports resolveViaWorkspace/parseBareSpecifier from TypeScript and adds a process-lifetime Mutex-guarded cache (workspace_resolved_cache) for workspace-resolved paths, reset once per build; core logic is correct and well-tested with 21 new unit tests, though mutex-poisoning on reset is silently ignored.
crates/codegraph-core/src/domain/graph/builder/pipeline.rs Threads the workspace map through pipeline_setup, resolve_pipeline_imports, reparse_barrel_candidates, and ImportEdgeContext; reset happens before Stage 6 resolution as intended.
crates/codegraph-core/src/lib.rs FFI entry points updated: resolve_imports (batch) correctly resets cache before resolving; resolve_import (single) does not reset, which is consistent with the documented design intent.
src/domain/graph/resolve.ts Adds getWorkspacesForNative() helper and wires workspace list into both resolveImportPath and resolveImportsBatch native calls.
tests/integration/issue-1927-native-workspace-resolution.test.ts New integration test correctly verifies import resolution and call edges; however, the confidence assertion (>= 0.95) is satisfied by the 1.0 branch (imp == target_file) rather than the workspace 0.95 floor branch for this specific fixture shape.
crates/codegraph-core/src/types.rs Adds WorkspacePackage with both #[napi(object)] and #[serde(rename_all = "camelCase")]; the dual-annotation reasoning is clearly documented.
src/domain/graph/builder/stages/native-orchestrator.ts One-line change: adds JSON.stringify(getWorkspacesForNative(this.ctx.rootDir)) as the fifth argument to the native buildGraph call.
src/types.ts Adds NativeWorkspacePackage interface and updates NativeAddon/NativeDatabase FFI signatures with optional workspaces parameters, maintaining backward compatibility.
crates/codegraph-core/src/db/connection.rs Adds optional workspaces_json parameter with backward-compatible Option unwrap-or-default; wired through to run_pipeline.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant JS as resolve.ts (JS)
    participant FFI as lib.rs (napi FFI)
    participant RES as resolve.rs (Rust)
    participant CACHE as workspace_resolved_cache (global Mutex)
    participant CONF as compute_confidence

    Note over JS,CACHE: Per-call FFI path (JS-driven build)

    JS->>JS: detectWorkspaces() → workspace map
    JS->>JS: getWorkspacesForNative(rootDir)
    JS->>FFI: resolveImports(inputs, rootDir, aliases, knownFiles, workspaces)
    FFI->>CACHE: reset_workspace_resolved_paths()
    FFI->>RES: resolve_imports_batch(..., Some(workspaces))
    loop par_iter over imports
        RES->>RES: resolve_import_path_inner()
        RES->>RES: resolve_non_relative_import() → resolve_via_workspace()
        RES->>CACHE: mark_workspace_resolved(rel_path)
    end
    RES-->>FFI: Vec ResolvedImport
    FFI-->>JS: resolved paths

    JS->>FFI: computeConfidence(callerFile, targetFile, importedFrom)
    FFI->>CONF: compute_confidence()
    CONF->>CACHE: is_workspace_resolved(imp)
    CACHE-->>CONF: true/false
    CONF-->>FFI: 0.95 (workspace floor) or proximity score
    FFI-->>JS: confidence

    Note over JS,CACHE: Rust orchestrator path (buildGraph)

    JS->>FFI: buildGraph(rootDir, configJson, aliasesJson, optsJson, workspacesJson)
    FFI->>FFI: pipeline_setup() → reset_workspace_resolved_paths()
    FFI->>RES: resolve_imports_batch(..., Some(workspaces))
    RES->>CACHE: mark_workspace_resolved(...)
    FFI->>CONF: compute_confidence() reads cache
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant JS as resolve.ts (JS)
    participant FFI as lib.rs (napi FFI)
    participant RES as resolve.rs (Rust)
    participant CACHE as workspace_resolved_cache (global Mutex)
    participant CONF as compute_confidence

    Note over JS,CACHE: Per-call FFI path (JS-driven build)

    JS->>JS: detectWorkspaces() → workspace map
    JS->>JS: getWorkspacesForNative(rootDir)
    JS->>FFI: resolveImports(inputs, rootDir, aliases, knownFiles, workspaces)
    FFI->>CACHE: reset_workspace_resolved_paths()
    FFI->>RES: resolve_imports_batch(..., Some(workspaces))
    loop par_iter over imports
        RES->>RES: resolve_import_path_inner()
        RES->>RES: resolve_non_relative_import() → resolve_via_workspace()
        RES->>CACHE: mark_workspace_resolved(rel_path)
    end
    RES-->>FFI: Vec ResolvedImport
    FFI-->>JS: resolved paths

    JS->>FFI: computeConfidence(callerFile, targetFile, importedFrom)
    FFI->>CONF: compute_confidence()
    CONF->>CACHE: is_workspace_resolved(imp)
    CACHE-->>CONF: true/false
    CONF-->>FFI: 0.95 (workspace floor) or proximity score
    FFI-->>JS: confidence

    Note over JS,CACHE: Rust orchestrator path (buildGraph)

    JS->>FFI: buildGraph(rootDir, configJson, aliasesJson, optsJson, workspacesJson)
    FFI->>FFI: pipeline_setup() → reset_workspace_resolved_paths()
    FFI->>RES: resolve_imports_batch(..., Some(workspaces))
    RES->>CACHE: mark_workspace_resolved(...)
    FFI->>CONF: compute_confidence() reads cache
Loading

Comments Outside Diff (1)

  1. tests/integration/issue-1927-native-workspace-resolution.test.ts, line 1252-1259 (link)

    P2 Confidence assertion doesn't reach the 0.95 workspace floor branch

    In the fixture, add and multiply both live in packages/lib/src/index.js — the same file that @myorg/lib resolves to. When compute_confidence iterates imported_from_set, it hits imp == target_file first and returns 1.0 before the is_workspace_resolved check fires. The assertion toBeGreaterThanOrEqual(0.95) passes (since 1.0 >= 0.95), but it never exercises the 0.95 workspace floor path.

    To actually cover that branch you'd need a fixture where the workspace package's entry file (index.js) re-exports symbols from a different internal file (e.g. packages/lib/src/math.js), so imp (index.js) ≠ target_file (math.js). The existing Rust unit test compute_confidence_returns_0_95_for_workspace_resolved_import does cover the branch correctly, so this is a gap only in the integration-level assertion.

    Fix in Claude Code

Fix All in Claude Code

Reviews (1): Last reviewed commit: "fix(native): resolve monorepo workspace ..." | Re-trigger Greptile

Comment on lines +278 to +282
pub fn reset_workspace_resolved_paths() {
if let Ok(mut set) = workspace_resolved_cache().lock() {
set.clear();
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Silent no-op on poisoned mutex can leave stale workspace paths across builds

If a thread panics while holding workspace_resolved_cache()'s lock, the mutex becomes poisoned and if let Ok(...) silently skips the clear(). Any subsequent build will then see workspace-resolved paths left over from the panicking build, causing compute_confidence to incorrectly award the 0.95 floor to paths that are no longer workspace-resolved. Using unwrap_or_else(|p| p.into_inner()) recovers the inner data from a poisoned guard rather than abandoning the clear entirely.

Suggested change
pub fn reset_workspace_resolved_paths() {
if let Ok(mut set) = workspace_resolved_cache().lock() {
set.clear();
}
}
pub fn reset_workspace_resolved_paths() {
let mut set = workspace_resolved_cache()
.lock()
.unwrap_or_else(|poisoned| poisoned.into_inner());
set.clear();
}

Fix in Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant